🦋 Changeset detected

Latest commit: 4b3b456

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-ui@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-utils@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-vite@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-event-bus@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/devtools-event-client@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/react-devtools@181

npm i https://pkg.pr.new/TanStack/devtools/@tanstack/solid-devtools@181

commit: 4b3b456

CC @TakhyunKim @colelawrence

Thank you for the PR, I'll review it in more details as soon as I can, before then, what exactly would be the benefit of rendering it in a shadow dom besides escaping global styling?

Of course, @AlemTuzlak!

I honestly haven't really thought about any other benefits but I guess this is what I found after a little research:

• Style/Event isolation (could be very important for a package of this nature)
• A core for web components (I see PR feat: add web components to devtools #52)
• Supports native PIP

Beyond that, I'm not sure what additional benefits we receive. In all honesty, I would hope style isolation would be an absolute requirement for this package if its intended to be used across any application. I've tried to introduce this into our organization, but our CSS reset is getting in the way of the styles in the devtools and things are quite visually broken.

As an FYI, I was able to end up binding the goober css function to render inside of the shadow node more idiomatically (using context throughout the codebase for css) but this only solves styles for devtools and devtools-ui. Unfortunately, additional packages like @tanstack/react-query-devtools and @tanstack/react-router-devtools just default to their current document context and still end up appending their styles to the head of the document instead of inside the shadow node.

All this to say, the current solution (in this PR) syncs styles from all goober usage on the page, to within the shadow root and it works perfectly fine. The code for the pip styling was inspiration for this.

This was the proof-of-concept for the context stuff if you cared to see, but it seems like it would be a pain to continue down this route.

// packages/devtools-ui/src/components/shadow-root.tsx

import { createContext, onCleanup, onMount, useContext } from 'solid-js'
import { render } from 'solid-js/web'
import { css as gooberCss } from 'goober'
import type { ParentComponent } from 'solid-js'

export type CssFn = typeof gooberCss
type GooberCtx = { css: CssFn }

const GooberContext = createContext<GooberCtx>()

export const useCss = () => {
  const ctx = useContext(GooberContext)
  if (!ctx) throw new Error('useCss must be used inside <ShadowRoot>')
  return ctx
}

export const ShadowRoot: ParentComponent = (props) => {
  let host!: HTMLDivElement

  onMount(() => {
    const shadow = host.attachShadow({ mode: 'open' })
    const ctx: GooberCtx = { css: gooberCss.bind({ target: shadow }) }

    const dispose = render(
      () => (
        <GooberContext.Provider value={ctx}>
          {props.children}
        </GooberContext.Provider>
      ),
      shadow,
    )
    onCleanup(dispose)
  })

  return <div ref={host} />
}

I had a question @jakiestfu - I already started using the DevTools to be a frame for some existing UI pieces I'm using in my app. Does this affect that use case? I'd rather not have to muck around with css injection or something to get around the shadow dom.

Do you mean this frame, @colelawrence? If so, I have no idea. I could explore your use-cases for you if you wanted to share a little more or high-level it for me.

MDN does note frame is a deprecated feature and Shadow DOM is shiny and new and stuff 🤷

And if by chance you mean iframe, that comes with its own complexities regarding postMessage and stuff. Unless they're on the same origin.

I'd rather not have to muck around with css injection

We already do that for the PIP window though because we're using Goober 🥺

I apologize in advance for my eagerness to have this shipped or explored by the maintainers. The lack of style sandboxing is fully preventing us at Turo from adopting devtools internally as the styles are well... broken

Stronger CSS styling could help this problem, but that pushes the work and effort unto the end-users of the devtools package as opposed to the package itself handling this.

What can I do for ya'll to help support your efforts?

Sorry I had some deadlines I needed to hit so I had no time to look at this, will get to it this week definitely

I've been looking at this, and I have no idea how it works with the change you did to the react-devtools adapter but here are a few things:

• i don't want this behavior to be the default one but rather a config option you can pass in, eg shadowDOMElement to mount to
• this also needs to be changed I guess with the title in the react-devtools

@AlemTuzlak Thank you kindly for your feedback. I'll address your comments:

I've been looking at this, and I have no idea how it works with the change you did to the react-devtools adapter but here are a few things:

If you are referring to this code:

If we render the devtools into a shadow node, const target = e.ownerDocument.getElementById(id) returns undefined instead of the container element to render the DOM node into.

Thing is, the container should always be present since thats being rendered by the devtools itself, so I removed that line so it allows the parent devtools to still render components whilst in the shadow node. I think this is just a potentially unnecessary safe-guard, but I could be wrong.

I don't want this behavior to be the default one but rather a config option you can pass in, eg shadowDOMElement to mount to

This is not sufficient because the way Goober is used, it just blindly renders styles to the parent HTML page. Thats why styles are (currently) synced from the main page to the PIP window, and why they'd need to be synced to the Shadow now as well. If the core devtools (and all subsequent plugins) supported more options about where to render things, it could work, but this should be a feature that should "just work" without end-users having to worry about the implications of how we render it.

I know that react-query-devtools supports passing a shadow DOM target but this would require quite a large refactor to support with the current devtools architecture. We'd need access to the container that elements need to mount into in the plugins array and things begin to feel messy.

This also needs to be changed I guess with the title in the react-devtools
Do you mind expanding upon this more? I'm not sure I quite follow.

All this to say, if you feel this is a bigger architectural/code change than you'd like to make right now, perhaps I can submit a PR that tightens up styles in the devtools so things aren't as visually broken for our use-cases. I just feat we'll always have to compete with styles from the outer page in the child components.